-
Notifications
You must be signed in to change notification settings - Fork 21
PROD-2740 Update loading spinner -> dev #248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…t from the dom; update all uses of the loading spinner so it shows/hides correctly #time 30m
|
@brooketopcoder I've run through all the pages and the spinner seems to work, but I'm not sure how it wasn't working before, so it's not clear to me if what I'm seeing is correct. Can you clarify what I should or shouldn't see -- how was it broken before? |
src-ts/tools/work/work-detail-details/work-detail-details-pane/WorkDetailDetailsPane.tsx
Outdated
Show resolved
Hide resolved
vas3a
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the LoadingSpinner's show property should default to true. This way when we add <LoadingSpinner /> it's visible (if it's hidden without specifying a param, eg show={false}, it is confusing).
I totally agree. Booleans should always default to false. I will make this fix... |
…; fix spinner on home page #time 30m
…d global full-height-frame for other loading spinners; #time 30m
|
@vas3a i made the changes you suggested and improved the loading UI in a couple places. I also added some error handling bc we are getting a 500 from the dev api when trying to get course progress. |
vas3a
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome!
https://topcoder.atlassian.net/browse/PROD-2740
The loading spinner was incorrectly implemented and needs to be refactored. Please verify the spinner correctly displays if the data doesn’t load right away in the following locations:
Learn
Work